Skip to content

feat(codecs): accept the encoders this ffmpeg has, and check them once up front - #23

Merged
rohanpoudel2 merged 1 commit into
mainfrom
feat/codec-parity
Aug 22, 2026
Merged

feat(codecs): accept the encoders this ffmpeg has, and check them once up front#23
rohanpoudel2 merged 1 commit into
mainfrom
feat/codec-parity

Conversation

@rohanpoudel2

Copy link
Copy Markdown
Owner

Second of three PRs for 2.1.0. Independent of #22 — different files, no
conflicts. PR 3 (--dry-run as a real plan) stacks on this one, because
"report capability failures in the plan" is the preflight added here.

The CLI could not say what the library and README already promised

$ imgvidcompress video --audio-codec libmp3lame ...
error: option '--audio-codec <name>' argument 'libmp3lame' is invalid.
       Allowed choices are aac, libopus, copy.

AUDIO_CODECS defines six. VIDEO_CONTAINERS['.mkv'].audio lists all six. The
README's own container table advertises MP3/Vorbis/FLAC for .mkv and MP3/FLAC
for .mp4. Only the CLI refused, via a hardcoded .choices() array.

Same story for --codec, pinned to the eight curated names — while README:206
claimed "around 100 video encoders available to --codec", and openArgsFor
already validated arbitrary encoders against caps.videoEncoders. That branch
was unreachable from the CLI.

Both choice lists are gone. Curated names remain as help-text suggestions, for
the reason addToOption() already documents for --to: what a build supports
is a property of the binary, so a closed list both rejects what works and
advertises what does not.

Validation was inverted

Uncurated containers were checked against the real encoder list. Curated ones —
the common ones — trusted only the static matrix. So --to .mp4 --codec libsvtav1 on a build without SVT-AV1 produced one wall of raw ffmpeg stderr
per file, and a build without libx264 failed every .mp4 job with no
explanation, because libx264 is the unchecked default.

A preflight now runs once, after ffmpeg resolves and before the worker pool
starts, over the distinct target formats the planned jobs need:

  • Missing explicitly requested encoder → the run fails immediately, naming
    what this build does have for that container.
  • Missing default encoder → an available legal codec is substituted and
    reported as a warning, not swapped silently.

The audio copy-preference is untouched. resolveAudioCodec still sees each
file's probed streams and prefers copy when every track can be carried; the
preflight only supplies the fallback used when re-encoding is genuinely needed.

Verified

mkv + --audio-codec libmp3lame   → compressed; ffprobe confirms mp3 written
.nut + --codec ffv1              → accepted, reaches ffmpeg (open tier)
.webm + --codec libx264          → "WebM cannot carry libx264. Supported: ..."
bad encoder over 3 input files   → 1 error, not 3

189 tests pass. typecheck (both packages) / lint / format:check /
build / build:mcp all clean.

MCP

Reads its real version instead of a hardcoded 0.1.0. list_capabilities
returns muxer and encoder names alongside the counts it already reported —
a count cannot be used to plan a conversion. Additive; existing fields kept.

Notes for review

  • schemaVersion stays 1. CompressionSummary.warnings and the run-start
    event's warnings are both optional additions.
  • One type-level change worth a look: CompressionJob.targetFormat widens from
    VideoContainer to VideoOutputSpec. The runtime has always been able to
    produce an arbitrary muxer extension here — the old type was simply wrong. A
    consumer exhaustively switching on it would now see a wider union.
  • Curated containers still enforce their matrix, so --to .mp4 --codec libx264rgb is rejected. That is deliberate — the matrix is verified and
    carries tuned per-codec flags — and the README now says so explicitly instead
    of implying every encoder is available everywhere.
  • Pre-existing, not touched here: openArgsFor derives the muxer name from the
    extension (.tsts), so formats whose muxer is named differently
    (mpegts) are rejected as unwritable. Worth a follow-up.

🤖 Generated with Claude Code

…e up front

The CLI pinned --codec and --audio-codec to hardcoded commander choice lists,
so it could not express what the library and the README already promised:

  $ imgvidcompress video --audio-codec libmp3lame ...
  error: Allowed choices are aac, libopus, copy.

AUDIO_CODECS defines six, VIDEO_CONTAINERS['.mkv'].audio lists all six, and the
README's container table advertises MP3/Vorbis/FLAC. Only the CLI refused. The
same applied to --codec, where the open-tier branch in compress.ts already
validated arbitrary encoders against ffmpeg but was unreachable from the CLI.

Both choice lists are gone. The curated names stay as help-text suggestions,
for the reason addToOption() already documents for --to: what a build supports
is a property of the binary, and a closed list both rejects what works and
advertises what does not.

Validation was also inverted. Uncurated containers checked
caps.videoEncoders.has(requested) and failed clearly; curated containers
trusted only the static matrix, so a build without libsvtav1 produced one raw
ffmpeg failure per file, and a build without libx264 failed every .mp4 job
with no explanation because libx264 is the unchecked default.

A preflight now runs once, after ffmpeg resolves and before the pool starts,
over the distinct target formats the planned jobs need. A missing explicitly
requested encoder fails the run immediately with the alternatives this build
does have. A missing default is substituted with an available legal codec and
reported rather than swapped silently.

The audio copy-preference is unchanged: resolveAudioCodec still sees the
probed source streams per file and prefers 'copy' when every track can be
carried. The preflight only supplies the fallback used when re-encoding is
actually required.

MCP reads its real version instead of a hardcoded 0.1.0, and list_capabilities
returns muxer and encoder names alongside the counts it already reported. A
count cannot be used to plan a conversion.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@rohanpoudel2
rohanpoudel2 merged commit e51b06f into main Aug 22, 2026
8 checks passed
rohanpoudel2 added a commit that referenced this pull request Aug 22, 2026
Version bumps, a changelog, and a clean audit.

The lockfile is regenerated alongside package.json. #21 existed because the
two drifted apart last release and npm ci refuses to run when they disagree;
verified here by wiping node_modules and running npm ci from scratch.

The MCP package goes to 0.2.0. It gained real version reporting and encoder
names in #23, and it reads its own package.json at runtime now, so the number
it reports is this one.

Dev-only advisories cleared: nanoid (high) via npm audit fix, esbuild (low)
via an override to ^0.28.2. Neither reached published output. The esbuild
issue affects its dev server, which this project never runs, but the override
is free and 0 findings is a better default than a documented exception.

CHANGELOG.md ships in the tarball.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant